From: Joshua Goins Date: Tue, 10 Feb 2026 15:30:00 +0000 (-0500) Subject: [PATCH] rhi: vulkan: Fix unintentional SDR format selection X-Git-Tag: archive/raspbian/6.10.2+dfsg-6+rpi1^2~8 X-Git-Url: https://dgit.raspbian.org/%22http:/www.example.com/%22mailto:kde%40ewsoftware.de//%22style.css/%22/%22http:/www.example.com/%22mailto:kde%40ewsoftware.de/%22style.css/%22?a=commitdiff_plain;h=e5f97770b8734b1cb4b460ef6c3b382ae7e2e106;p=qt6-base.git [PATCH] rhi: vulkan: Fix unintentional SDR format selection I needed to read back the swapchain image for my Vulkan RHI-enabled QtQuick application, but it complained that VK_FORMAT_A2R10G10B10_UNORM_PACK32 wasn't able to be read back - why? My initial naive solution is to simply add it to the swapchainReadbackTextureFormat function - but that wouldn't work as there is no BGR10A2 RHI texture format and applications would unkowingly end up with swapped channels. The actual problem came down to how we were selecting swapchain image formats. The first thing I changed was the hdrFormatMatchesVkSurfaceFormat function, because that checked two formats for HDR10: VK_FORMAT_A2B10G10R10_UNORM_PACK32 and VK_FORMAT_A2R10G10B10_UNORM_PACK32. Checking the online Vulkan hardware database, the BGR variant is more well-supported. Picking the RGB variant is inevitably going to lead into the problem described before - and should be added back when & if a new RHI texture format is introduced. The next thing I changed was the swapchain format selection logic, specifically the choice for a non-sRGB SDR format. Judging by the comments in this function and other RHIs like DX12 we *want* the default color format of VK_FORMAT_B8G8R8A8_UNORM unless otherwise requested. That isn't what was happening though, on my specific hardware it was choosing VK_FORMAT_A2R10G10B10_UNORM_PACK32 - why? It comes down to the isSrgbFormat check in the loop. Again, for the non-SRGB SDR case the "srgbRequested" variable is always false. And when a non-SRGB format (like the aforementioned problematic VkFormat) is checked isSrgbFormat will return false, but I don't think that's what is intended here. We want that for the sRGB case, but for non-SRGB SDR the default color format is fine and that lines up with other RHIs (see QD3D12SwapChain::chooseFormats for an example.) I checked this inside the loop so the passthrough code is still ran on Wayland, but I think the new logic is still sensible. I tested this against the five usual cases I could think of and now the format selection seems sensible: * non-sRGB SDR chose VK_FORMAT_B8G8R8A8_UNORM * sRGB SDR chose VK_FORMAT_R8G8B8A8_SRGB * extended sRGB Linear chose VK_FORMAT_R16G16B16A16_SFLOAT * HDR10 chose VK_FORMAT_A2B10G10R10_UNORM_PACK32 * Display P3 chose VK_FORMAT_R16G16B16A16_SFLOAT Change-Id: Ie79e9fcaa1130311958b485af9b73c59d5d9a335 Reviewed-by: Laszlo Agocs (cherry picked from commit 2dd1aa3678d541aef15b564b4013728ed5b0387b) Reviewed-by: Qt Cherry-pick Bot (cherry picked from commit f4c5e54a8703944ec7ea005ad4de3072b86fd61f) Gbp-Pq: Name upstream_hdr_vulkan.diff --- diff --git a/src/gui/rhi/qrhivulkan.cpp b/src/gui/rhi/qrhivulkan.cpp index a246c86a1..12acbaee1 100644 --- a/src/gui/rhi/qrhivulkan.cpp +++ b/src/gui/rhi/qrhivulkan.cpp @@ -8691,7 +8691,7 @@ static inline bool hdrFormatMatchesVkSurfaceFormat(QRhiSwapChain::Format f, cons return s.format == VK_FORMAT_R16G16B16A16_SFLOAT && s.colorSpace == VK_COLOR_SPACE_EXTENDED_SRGB_LINEAR_EXT; case QRhiSwapChain::HDR10: - return (s.format == VK_FORMAT_A2B10G10R10_UNORM_PACK32 || s.format == VK_FORMAT_A2R10G10B10_UNORM_PACK32) + return s.format == VK_FORMAT_A2B10G10R10_UNORM_PACK32 && s.colorSpace == VK_COLOR_SPACE_HDR10_ST2084_EXT; case QRhiSwapChain::HDRExtendedDisplayP3Linear: return s.format == VK_FORMAT_R16G16B16A16_SFLOAT @@ -8810,37 +8810,81 @@ bool QVkSwapChain::ensureSurface() quint32 formatCount = 0; rhiD->vkGetPhysicalDeviceSurfaceFormatsKHR(rhiD->physDev, surface, &formatCount, nullptr); QList formats(formatCount); - if (formatCount) - rhiD->vkGetPhysicalDeviceSurfaceFormatsKHR(rhiD->physDev, surface, &formatCount, formats.data()); + rhiD->vkGetPhysicalDeviceSurfaceFormatsKHR(rhiD->physDev, surface, &formatCount, + formats.data()); - // See if there is a better match than the default BGRA8 format. (but if - // not, we will stick to the default) + // Initially select the first available format, will only be used as a worst-case fallback + colorFormat = formats.constFirst().format; + colorSpace = formats.constFirst().colorSpace; + + // See if we can find the preferred SDR format const bool srgbRequested = m_flags.testFlag(sRGB); - for (int i = 0; i < int(formatCount); ++i) { - if (formats[i].format != VK_FORMAT_UNDEFINED) { - bool ok = srgbRequested == isSrgbFormat(formats[i].format); - if (m_format != SDR) - ok &= hdrFormatMatchesVkSurfaceFormat(m_format, formats[i]); + bool foundBestFormat = false; + if (m_format == SDR) { + for (int i = 0; i < int(formatCount); ++i) { + bool ok; + if (srgbRequested) { + ok = defaultSrgbColorFormat == formats[i].format; + } else { + ok = defaultColorFormat == formats[i].format; + } if (ok) { + foundBestFormat = true; colorFormat = formats[i].format; colorSpace = formats[i].colorSpace; -#if QT_CONFIG(wayland) - // On Wayland, only one color management surface can be created at a time without - // triggering a protocol error, and we create one ourselves in some situations. - // To avoid this problem, use VK_COLOR_SPACE_PASS_THROUGH_EXT when supported, - // so that the driver doesn't create a color management surface as well. - const bool hasPassThrough = std::any_of(formats.begin(), formats.end(), [this](const VkSurfaceFormatKHR &fmt) { - return fmt.format == colorFormat && fmt.colorSpace == VK_COLOR_SPACE_PASS_THROUGH_EXT; - }); - if (hasPassThrough) { - colorSpace = VK_COLOR_SPACE_PASS_THROUGH_EXT; - } -#endif break; } } } + // Otherwise pick one that fits our requirements: + if (!foundBestFormat && (m_format != SDR || srgbRequested)) { + for (int i = 0; i < int(formatCount); ++i) { + bool ok = false; + if (m_format != SDR) { + ok = hdrFormatMatchesVkSurfaceFormat(m_format, formats[i]); + } else if (srgbRequested) { + ok = isSrgbFormat(formats[i].format); + } + if (ok) { + colorFormat = formats[i].format; + colorSpace = formats[i].colorSpace; + break; + } + } + } + + // Although this should be rare, warn when we fail to select a suitable format + if (m_format != SDR) { + VkSurfaceFormatKHR format{}; + format.format = colorFormat; + format.colorSpace = colorSpace; + if (!hdrFormatMatchesVkSurfaceFormat(m_format, format)) { + qWarning("Failed to select a suitable VkFormat for HDR, using format %d as fallback", + colorFormat); + } + } else { + if (srgbRequested && !isSrgbFormat(colorFormat)) { + qWarning("Failed to select a suitable VkFormat for sRGB, using format %d as fallback", + colorFormat); + } + } + +#if QT_CONFIG(wayland) + // On Wayland, only one color management surface can be created at a time without + // triggering a protocol error, and we create one ourselves in some situations. + // To avoid this problem, use VK_COLOR_SPACE_PASS_THROUGH_EXT when supported, + // so that the driver doesn't create a color management surface as well. + const bool hasPassThrough = + std::any_of(formats.begin(), formats.end(), [this](const VkSurfaceFormatKHR &fmt) { + return fmt.format == colorFormat + && fmt.colorSpace == VK_COLOR_SPACE_PASS_THROUGH_EXT; + }); + if (hasPassThrough) { + colorSpace = VK_COLOR_SPACE_PASS_THROUGH_EXT; + } +#endif + samples = rhiD->effectiveSampleCountBits(m_sampleCount); quint32 presModeCount = 0; diff --git a/src/gui/rhi/qrhivulkan_p.h b/src/gui/rhi/qrhivulkan_p.h index 59d98c309..17ada4f99 100644 --- a/src/gui/rhi/qrhivulkan_p.h +++ b/src/gui/rhi/qrhivulkan_p.h @@ -625,7 +625,9 @@ struct QVkSwapChain : public QRhiSwapChain int bufferCount = 0; VkSurfaceKHR surface = VK_NULL_HANDLE; VkSurfaceKHR lastConnectedSurface = VK_NULL_HANDLE; - VkFormat colorFormat = VK_FORMAT_B8G8R8A8_UNORM; + const VkFormat defaultColorFormat = VK_FORMAT_B8G8R8A8_UNORM; + const VkFormat defaultSrgbColorFormat = VK_FORMAT_B8G8R8A8_SRGB; + VkFormat colorFormat = VK_FORMAT_UNDEFINED; VkColorSpaceKHR colorSpace = VK_COLOR_SPACE_SRGB_NONLINEAR_KHR; QVkRenderBuffer *ds = nullptr; VkSampleCountFlagBits samples = VK_SAMPLE_COUNT_1_BIT;